Skip to content

Formatting files in /source/source_hamilt/module_xc, try to remove GlobalC::exx_info#7399

Open
mohanchen wants to merge 22 commits into
deepmodeling:developfrom
mohanchen:20260530
Open

Formatting files in /source/source_hamilt/module_xc, try to remove GlobalC::exx_info#7399
mohanchen wants to merge 22 commits into
deepmodeling:developfrom
mohanchen:20260530

Conversation

@mohanchen
Copy link
Copy Markdown
Collaborator

@mohanchen mohanchen commented May 29, 2026

Formatting files in /source/source_hamilt/module_xc

The global class GlobalC::exx_info should have been removed two years ago. However, other developers were not aware of this issue and kept extending its functionalities, leaving persistent risks in the code for exchange-correlation functionals. Flawed code tends to cause exponentially escalating problems over time. For this reason, I have refactored relevant modules to eliminate GlobalC::exx_info.

abacus_fixer added 2 commits May 30, 2026 06:32
- Add tau_xc overload with hybrid_alpha parameter
- Add ElecState::set_exx overload with cal_exx and hybrid_alpha parameters
- Format code: one parameter per line, align code, initialize variables, one variable per line, add braces to single-line for/if
@mohanchen mohanchen added Refactor Refactor ABACUS codes The Absolute Zero Reduce the "entropy" of the code to 0 labels May 29, 2026
abacus_fixer added 4 commits May 30, 2026 07:15
- Format function parameters to one per line
- Initialize all variables when declared
- Separate variable declarations onto individual lines
- Add braces for single-line if/for statements
- Improve code alignment and readability
@mohanchen mohanchen requested a review from zzlinpku May 30, 2026 01:52
Copy link
Copy Markdown
Collaborator

@zzlinpku zzlinpku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

This is a well-executed code style refactoring PR. The new file naming scheme (xc_*.cpp for native implementations, libxc_*.cpp for Libxc wrappers) is clear and consistent. A few items to address:

🔴 Must fix: Header guard in libxc.h still uses the old name.
🟡 Consider: Update comment references to old file names, and note the reference member risk in exx_info.h.

Comment thread source/source_hamilt/module_xc/libxc.h Outdated
Comment thread source/source_hamilt/module_xc/libxc.h Outdated
Comment thread source/source_hamilt/module_xc/libxc.h Outdated
Comment thread source/source_hamilt/module_xc/libxc.h Outdated
Comment thread source/source_hamilt/module_xc/libxc.h Outdated
Comment thread source/source_hamilt/module_xc/libxc.h Outdated
Comment thread source/source_hamilt/module_xc/xc_functional.h Outdated
Comment thread source/source_hamilt/module_xc/xc_functional.h
Comment thread source/source_hamilt/module_xc/libxc_mgga_wrap.cpp Outdated
Comment thread source/source_hamilt/module_xc/exx_info.h Outdated
Copy link
Copy Markdown
Collaborator

@zzlinpku zzlinpku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

This is a well-structured refactoring PR. The file renames are consistent and improve readability, the indentation cleanup is thorough, and the exx_info.h lifetime warning is valuable.

However, there are two issues that should be addressed before merging — one potential name collision risk and one concern about mixing behavioral changes with formatting.

See inline comments for details.

Comment thread source/source_hamilt/module_xc/libxc.h Outdated
Comment thread source/source_hamilt/module_xc/libxc_mgga_wrap.cpp
Comment thread source/source_hamilt/module_xc/xc_gga_corr.cpp Outdated
Comment thread source/source_hamilt/module_xc/exx_info.h Outdated
Comment thread source/source_hamilt/module_xc/xc_grad.cpp
abacus_fixer added 8 commits June 7, 2026 11:20
This is the first step of removing GlobalC::exx_info global variable.

## Changes
- Split Exx_Info into 4 independent header files:
  - exx_info_global.h: Exx_Info_Global (core global settings)
  - exx_info_lip.h: Exx_Info_Lip (LIP method parameters)
  - exx_info_ri.h: Exx_Info_RI (RI method parameters)
  - exx_info_opt_abfs.h: Exx_Info_Opt_ABFs (optimized ABF parameters)
- Modified exx_info.h to include the 4 new headers
- Updated all references from Exx_Info::Exx_Info_* to Exx_Info_*

## Benefits
1. Reduced coupling: modules can now include only needed headers
2. Types are now independent, preparing for parameter passing
3. GlobalC::exx_info remains unchanged, fully backward compatible

## Next Steps (Phase 2-3)
Phase 2: Replace references with value copies in Exx_Info_Lip/RI
  - Exx_Info_Lip: ccp_type& and hse_omega& -> value copies
  - Exx_Info_RI: coulomb_param& -> value copy
  - This removes dependency on Exx_Info_Global lifetime

Phase 3: Remove GlobalC::exx_info global variable
  - Pass Exx_Info_* as parameters instead of global access
  - Update ~60 files that use GlobalC::exx_info
  - Modules: module_xc, module_ri, source_lcao, source_esolver, source_pw
…ip/RI

## Changes

### 1. Modified struct definitions

**`exx_info_lip.h`**: Changed references to value copies
- `const Ccp_Type& ccp_type` → `Ccp_Type ccp_type`
- `const double& hse_omega` → `double hse_omega = 0.11`
- Removed constructor dependency on Exx_Info_Global

**`exx_info_ri.h`**: Changed reference to value copy
- `const std::map<...>& coulomb_param` → `std::map<...> coulomb_param`
- Removed constructor dependency on Exx_Info_Global

### 2. Added sync method in exx_info.h

```cpp
void sync_from_global()
{
    info_lip.ccp_type = info_global.ccp_type;
    info_lip.hse_omega = info_global.hse_omega;
    info_ri.coulomb_param = info_global.coulomb_param;
}
```

### 3. Added sync calls

| File | Location | Purpose |
|------|----------|----------|
| `input_conv.cpp` | Line 520 | After EXX parameter initialization |
| `esolver_lrtd_lcao.cpp` | Lines 266, 404 | After setting ccp_type |
| `RPA_LRI.hpp` | Line 148 | After setting ccp_type |

## Benefits

1. **Eliminated lifetime dependency**: Exx_Info_Lip and Exx_Info_RI are now independent
2. **Safe to copy/move**: No dangling reference risks
3. **Ready for parameter passing**: Structs can be safely passed as function parameters
4. **Fully backward compatible**: GlobalC::exx_info still exists and works unchanged

## Next Steps (Phase 3)

Remove GlobalC::exx_info global variable by:
- Passing Exx_Info_* structs as parameters
- Updating ~60 files that access GlobalC::exx_info
- Modules: module_xc, module_ri, source_lcao, source_esolver, source_pw
@mohanchen mohanchen changed the title Formatting files in /source/source_hamilt/module_xc Formatting files in /source/source_hamilt/module_xc, try to remove GlobalC::exx_info Jun 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Refactor Refactor ABACUS codes The Absolute Zero Reduce the "entropy" of the code to 0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants